-
Notifications
You must be signed in to change notification settings - Fork 325
action_sheet: Add 'Channel feed' button to channel action sheet #1794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
action_sheet: Add 'Channel feed' button to channel action sheet #1794
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! All LGTM except two small comments below; then please go ahead and merge.
test/widgets/action_sheet_test.dart
Outdated
testWidgets('from subscription list: visible ', (tester) async { | ||
await prepare(); | ||
await showFromInbox(tester); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-paste error? 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch!
test/widgets/action_sheet_test.dart
Outdated
await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e | ||
} | ||
|
||
testWidgets('from inbox: visible ', (tester) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: stray space (here and others below)
testWidgets('from inbox: visible ', (tester) async { | |
testWidgets('from inbox: visible', (tester) async { |
1d89611
to
c47538c
Compare
Thanks, revision pushed! I've added one more commit, PTAL: c47538c topic_list: Don't show topic-list button in action sheet from app bar |
c47538c
to
add8dc5
Compare
Ah pushed again to fix a failing test in that last commit. |
Thanks! Looks good; go ahead and merge once CI passes. |
…setup It looks like this param was only being used to avoid the topic-autocomplete behavior mentioned in the code comment. For invoking the channel action sheet by long-pressing the app bar, it shouldn't matter if the message list isn't showing any messages.
Similarly to how we don't show the "Channel feed" button when opening the same action sheet from a channel feed.
add8dc5
to
e12673a
Compare
Thanks! Done, after rebasing and running |
Fixes #1705.
Screenshots
From channel feed (no button)
Not from channel feed (button)